Skip to content

fix(sanctions): pass timeRange filter to sanctions pressure API#2455

Open
Jayanth-reflex wants to merge 13 commits intokoala73:mainfrom
Jayanth-reflex:fix/sanctions-timerange-filter
Open

fix(sanctions): pass timeRange filter to sanctions pressure API#2455
Jayanth-reflex wants to merge 13 commits intokoala73:mainfrom
Jayanth-reflex:fix/sanctions-timerange-filter

Conversation

@Jayanth-reflex
Copy link
Copy Markdown

@Jayanth-reflex Jayanth-reflex commented Mar 28, 2026

Summary

  • Adds time_range query parameter to ListSanctionsPressureRequest proto and generated client/server
  • Server handler recomputes newEntryCount (and per-country/per-program counts) by comparing each entry's effectiveAt against Date.now() - windowMs, so the "New" badge accurately reflects the user-selected time window (1h, 6h, 24h, 48h, 7d)
  • Client service forwards timeRange from the app context to the API call
  • Sanctions panel reloads when the user changes the time range selector

Root cause

The sanctions panel never passed timeRange to its API endpoint. The server returned the seed script's diff-based isNew flags regardless of the UI filter, causing newEntryCount to always show 0 when no entries had been added since the last seed run.

Files changed

File What changed
proto/.../list_sanctions_pressure.proto Added time_range field (field 2)
src/generated/client/.../service_client.ts Added timeRange to request interface + query param serialization
src/generated/server/.../service_server.ts Added timeRange to request interface + query param parsing
server/.../list-sanctions-pressure.ts applyTimeRangeFilter() — recomputes isNew, newEntryCount, country & program counts based on effectiveAt within the time window
src/services/sanctions-pressure.ts fetchSanctionsPressure() now accepts optional timeRange param
src/app/data-loader.ts Passes this.ctx.currentTimeRange to fetchSanctionsPressure()
src/app/panel-layout.ts Added onTimeRangeChanged callback; triggers sanctions reload on range change
src/App.ts Wires onTimeRangeChanged to dataLoader.loadSanctionsPressure()

Test plan

  • Set timeRange to 7d → verify API call includes ?time_range=7d
  • Confirm newEntryCount reflects entries with effectiveAt within the last 7 days (not always 0)
  • Switch timeRange from 7d to 24h → panel reloads with updated counts
  • Set timeRange to all or omit → verify existing behavior (no filtering)
  • Verify per-country and per-program newEntryCount badges update correctly
  • Run npm run typecheck:all — passes cleanly

Fixes #2437

🤖 Generated with Claude Code

The sanctions panel was ignoring the user-selected timeRange filter,
causing newEntryCount to always show 0. The API endpoint never received
the timeRange parameter, so it could not filter entries by their
effectiveAt timestamp.

Changes:
- Add time_range query param to ListSanctionsPressureRequest proto
- Update generated client/server to pass and parse time_range
- Server handler now recomputes isNew, newEntryCount, and per-country/
  per-program counts based on the requested time window
- Client service accepts and forwards timeRange to the API
- Data loader passes ctx.currentTimeRange to fetchSanctionsPressure
- Panel layout triggers sanctions reload on timeRange change

Fixes koala73#2437

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 28, 2026

@Jayanth-reflex is attempting to deploy a commit to the Elie Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions bot added the trust:caution Brin: contributor trust score caution label Mar 28, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 28, 2026

Greptile Summary

This PR fixes the root cause of the always-zero newEntryCount badge in the sanctions panel by plumbing timeRange from the UI context all the way through to the API, and adds server-side logic to recompute isNew / per-country / per-program counts based on effectiveAt within the selected window. The wiring changes in App.ts, panel-layout.ts, data-loader.ts, the proto, and the generated stubs are all clean and correct. Two issues in the client-side service layer need attention before the feature works reliably end-to-end:

  • Circuit breaker cache is not keyed on timeRange (src/services/sanctions-pressure.ts). The breaker.execute() call omits a cacheKey, so all time-range variants share the same 30-minute cache slot. After the first fetch (e.g. 7d) is cached, switching to 1h silently returns the stale 7d data. AGENTS.md requires request-varying params to be part of the cache key.
  • Hydrated bootstrap data bypasses timeRange (src/services/sanctions-pressure.ts). getHydratedData is consume-once, but on the very first call the timeRange argument is ignored and the seed-script's static isNew flags are returned as-is. Users with a non-default time range persisted from a previous session will see incorrect counts on initial load.
  • Country/program entries can be silently dropped (server/.../list-sanctions-pressure.ts). countryMap is built by iterating entry countryCodes; any country in data.countries that has no matching entry in the current dataset is omitted from the filtered response when countryMap.size > 0. A safer approach is to start from data.countries and patch in the recomputed counts.

Confidence Score: 4/5

The server-side filtering logic is sound, but two client-side caching bugs will cause incorrect data to be displayed when switching time ranges — the feature's primary user path.

There are two P1 defects in sanctions-pressure.ts: (1) the circuit breaker cache is not keyed on timeRange, so switching windows silently serves stale data from the previous window; (2) bootstrap hydrated data bypasses the timeRange filter on first render. Both directly break the core behaviour this PR is meant to fix. Score 4 to indicate these should be resolved before merging.

src/services/sanctions-pressure.ts — circuit breaker cacheKey and hydration bypass both need fixes here.

Important Files Changed

Filename Overview
src/services/sanctions-pressure.ts Two P1 bugs: circuit breaker cache not keyed on timeRange (stale data on window switch), and hydrated bootstrap data bypasses timeRange entirely on first load.
server/worldmonitor/sanctions/v1/list-sanctions-pressure.ts New applyTimeRangeFilter correctly recomputes isNew and newEntryCount from effectiveAt; countries/programs absent from any entry's countryCodes are silently dropped from the filtered response (P2).
src/app/panel-layout.ts Adds onTimeRangeChanged callback to the debounced time-range handler; clean wiring with optional chaining.
src/app/data-loader.ts Passes ctx.currentTimeRange to fetchSanctionsPressure — correct one-line change.
src/App.ts Wires onTimeRangeChanged to dataLoader.loadSanctionsPressure; void-wraps the async call correctly.
src/generated/client/worldmonitor/sanctions/v1/service_client.ts Adds timeRange to request interface and query param serialization; correctly skips the param when empty string.
src/generated/server/worldmonitor/sanctions/v1/service_server.ts Adds timeRange parsing from query param with correct empty-string default.
proto/worldmonitor/sanctions/v1/list_sanctions_pressure.proto Adds optional time_range string field (field 2) with proper HTTP query annotation.

Comments Outside Diff (2)

  1. src/services/sanctions-pressure.ts, line 151-156 (link)

    P1 Hydrated bootstrap data bypasses timeRange filter

    getHydratedData is a consume-once function (it deletes the entry after the first read). On the very first call to fetchSanctionsPressure, if the bootstrap hydration cache is populated and currentTimeRange is already set (e.g. restored from user preferences), the hydrated payload is returned without applying any time-range filtering. The timeRange argument is silently ignored, and the panel displays the seed-script's static isNew flags rather than the window-specific counts.

    Because the hydration is consumed immediately, the next call (e.g. a scheduled refresh) will go through the circuit breaker path and return the correctly filtered data — but the initial render is stale for users with a non-default time range.

    Consider skipping the hydration path when timeRange is non-empty so the circuit breaker handles the filter:

    const hydrated = getHydratedData('sanctionsPressure') as ListSanctionsPressureResponse | undefined;
    if (hydrated?.entries?.length || hydrated?.countries?.length || hydrated?.programs?.length) {
      if (!timeRange) {
        const result = toResult(hydrated);
        latestSanctionsPressureResult = result;
        return result;
      }
      // Fall through to breaker path so the time-range filter is applied
    }
  2. src/services/sanctions-pressure.ts, line 158-174 (link)

    P1 Circuit breaker cache not keyed on timeRange

    breaker.execute() is called without a cacheKey option, so every call — regardless of timeRange — lands on the same internal default cache slot (30-minute TTL). The first successful fetch (e.g. timeRange='7d') populates that slot; any subsequent call with a different window (e.g. timeRange='1h') hits the same stale cached entry and silently returns wrong data.

    AGENTS.md explicitly states: "Cache key MUST include request-varying params".

    The fix is to pass the timeRange value as the cacheKey in the options object passed to breaker.execute, so each distinct time range gets its own cache slot.

    Context Used: AGENTS.md (source)

Reviews (1): Last reviewed commit: "fix(sanctions): pass timeRange filter to..." | Re-trigger Greptile

…ntryCount

The previous approach built countryMap/programMap by iterating entries,
which silently dropped countries or programs not referenced by any
entry's countryCodes/programs arrays. Now we start from the original
data.countries and data.programs arrays and only patch newEntryCount,
guaranteeing every item from the seed dataset is preserved.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Jayanth-reflex
Copy link
Copy Markdown
Author

Good catch — fixed in b92b46d.

The approach now starts from data.countries / data.programs as the base and only patches newEntryCount with recomputed values, so every country and program from the original dataset is always preserved regardless of what entry.countryCodes references.

Before (buggy):

// Built map from entries → looked up in data.countries → silently dropped unmatched
const countryMap = new Map();
for (const entry of retagged) { ... }

After (fixed):

// Start from original data, patch in new counts
const countryNewCounts = new Map();
for (const entry of retagged) { if (entry.isNew) ... }
const countries = (data.countries ?? []).map(c => ({
  ...c, newEntryCount: countryNewCounts.get(c.countryCode) ?? 0,
}));

Same pattern applied to programs.

…n when filtered

Two P1 client-side bugs:

1. Cache keyed on timeRange: breaker.execute() was called without a
   cacheKey, so all time-range variants shared the same 30-min cache
   slot. Switching from '7d' to '1h' silently returned stale '7d' data.
   Fix: pass cacheKey = timeRange || 'all' so each window has its own
   slot, and forward the same key to breaker.clearCache().

2. Bootstrap hydration bypasses timeRange: getHydratedData returns the
   seed script's static isNew flags and cannot be re-filtered. With a
   non-default timeRange the hydrated path was returning incorrect counts
   on initial render. Fix: skip the hydration path entirely when a
   timeRange is set, falling through to the circuit-breaker path which
   sends timeRange to the server and gets correctly filtered data.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Jayanth-reflex
Copy link
Copy Markdown
Author

Both P1 issues fixed in 41a24a8:

1. Circuit breaker cache keyed on timeRange

Each distinct window now gets its own cache slot:

const cacheKey = timeRange || 'all';
return breaker.execute(fn, emptyResult, {
  cacheKey,
  shouldCache: (result) => result.totalCount > 0,
});

clearCache() also receives the same key so only the relevant slot is evicted on empty responses.

2. Bootstrap hydration bypasses timeRange

The hydration path is now skipped when a timeRange is set. The seed-script's static isNew flags can't be re-filtered client-side, so we fall through to the circuit-breaker path which sends timeRange to the server and gets correctly computed counts back:

if (!timeRange) {
  const hydrated = getHydratedData('sanctionsPressure') ...
  if (hydrated?.entries?.length || ...) {
    // only reached when no filter — static flags are valid
    return toResult(hydrated);
  }
}
// timeRange set → always fetch live with the filter applied

@Jayanth-reflex
Copy link
Copy Markdown
Author

@koala73 , kindly review this PR

SebastienMelki
SebastienMelki previously approved these changes Mar 30, 2026
Copy link
Copy Markdown
Collaborator

@SebastienMelki SebastienMelki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well-reasoned fix. Author self-corrected all three Greptile P1s (circuit breaker cache key on timeRange, hydration bypass, country/program preservation). Proto → generated stubs → service → panel wiring is clean. LGTM.

@Jayanth-reflex
Copy link
Copy Markdown
Author

@SebastienMelki , could you please guide me on how I can get the pending checks done
image

@SebastienMelki
Copy link
Copy Markdown
Collaborator

@SebastienMelki , could you please guide me on how I can get the pending checks done image

I just launched the workflows, they should run in a few seconds

@Jayanth-reflex
Copy link
Copy Markdown
Author

@SebastienMelki , could you please guide me on how I can get the pending checks done image

I just launched the workflows, they should run in a few seconds

Thanks, @SebastienMelki. I see the Vercel check is failing. Could you please suggest how we can fix this?

@Jayanth-reflex
Copy link
Copy Markdown
Author

Hi @SebastienMelki, @koala73, could you please help me with the Vercel check and retrigger the workflows, so I can merge this PR?

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 2, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
worldmonitor Ready Ready Preview, Comment Apr 2, 2026 9:31pm

Request Review

Copy link
Copy Markdown
Owner

@koala73 koala73 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review — sanctions timeRange filter

Good direction on this PR — the timeRange filter is the right UX improvement. A few issues to address before merge.


🔴 P1 — Date.now() frozen in CDN cache

File: server/worldmonitor/sanctions/v1/list-sanctions-pressure.ts:55

applyTimeRangeFilter calls Date.now() inside a handler registered as 'static' tier in gateway.ts (s-maxage=600, stale-while-revalidate=300). The cutoff timestamp is evaluated once at cache-fill time, then frozen for the full 900s TTL.

For the 1h window: an entry with effectiveAt = T-3500s (30s inside the window at fill time) shows isNew=true in the cached response. At T=3600s that entry is now outside the window — users continue receiving the stale isNew=true badge for up to 15 minutes.

Preferred fix (aligns with gold standard): Move applyTimeRangeFilter logic into scripts/seed-sanctions-pressure.mjs. Write separate Redis keys: sanctions:pressure:v1:1h, sanctions:pressure:v1:6h, etc. Edge handler reads sanctions:pressure:v1:${req.timeRange || ''} — no Date.now() on edge, full CDN cacheability preserved.

Minimal fix: Return no-store cache headers for ?time_range=* requests in gateway.ts. Every filtered request hits the edge but getCachedJson still coalesces Redis reads so cost is low.


🔴 P1 — timeRange required in generated interfaces

Files: src/generated/client/worldmonitor/sanctions/v1/service_client.ts:5, src/generated/server/worldmonitor/sanctions/v1/service_server.ts:5

// Current (wrong):
timeRange: string

// Fix:
timeRange?: string

Proto semantics for an absent query param is an empty string default — the field is logically optional. Any call site constructing ListSanctionsPressureRequest without timeRange silently violates the contract.

Also in list-sanctions-pressure.ts:57:

// Current (dead code — effectiveAt is always string, else branch never executes):
const ts = typeof e.effectiveAt === 'string'
  ? Number(e.effectiveAt)
  : (e.effectiveAt as unknown as number);

// Fix:
const ts = Number(e.effectiveAt);

🟡 P2 — onTimeRangeChanged fires for all panels unconditionally

Files: src/App.ts, src/app/panel-layout.ts:~142

onTimeRangeChanged is wired into applyTimeRangeFilterDebounced which fires for all panel time range changes across the app. Every time range change on any panel (news, market, aviation, etc.) triggers loadSanctionsPressure() even when the sanctions panel is not mounted.

// Current:
onTimeRangeChanged: () => {
  void this.dataLoader.loadSanctionsPressure();
}

// Fix:
onTimeRangeChanged: () => {
  if (this.panelLayout.isPanelConnected('sanctions-pressure')) {
    void this.dataLoader.loadSanctionsPressure();
  }
}

🟡 P2 — Shared circuit breaker failure state across all time-range slots

File: src/services/sanctions-pressure.ts, src/utils/circuit-breaker.ts

The CircuitBreaker maintains a single CircuitState (failure counter + cooldownUntil). All 6 cache keys ('all', '1h', '6h', '24h', '48h', '7d') share that one counter. Two 7d failures puts the entire circuit into 5-minute cooldown, blocking the default 'all' path used as the canonical view.

persistCache: true and 30-min TTL limit the blast radius for returning users, but fresh sessions (or after IndexedDB eviction) will see error state across all views.

Fix: Use separate new CircuitBreaker(...) instances per time range in sanctions-pressure.ts for independent failure counters.

P1 — Date.now() frozen in CDN cache:
  The sanctions endpoint is tier 'static' (s-maxage=600). When
  time_range is present, applyTimeRangeFilter calls Date.now() to
  compute a cutoff that becomes stale once the CDN caches the response.
  Fix: set X-No-Cache header on the Response when time_range is present
  so the gateway emits no-store instead of s-maxage=600.

P1 — timeRange required in generated interfaces:
  Proto semantics for an absent query param is empty-string default, so
  the field is logically optional. Changed to timeRange?: string in both
  generated client and server interfaces. Also simplified dead-code
  branch in effectiveAt parsing (typeof check on a value that is always
  string from proto) to plain Number(e.effectiveAt).

P2 — onTimeRangeChanged fires unconditionally:
  Every time-range change across all panels triggered
  loadSanctionsPressure(), even when the sanctions panel was not
  mounted. Now guarded with a panels['sanctions-pressure'] check.

P2 — Shared circuit breaker failure state:
  All time-range cache keys shared a single CircuitState, so two
  failures on e.g. '7d' put the entire breaker into 5-min cooldown,
  blocking the canonical 'all' path. Fix: use a dedicated
  filteredBreaker for time-range requests with independent failure
  counters (shorter TTL, no persistent cache since filtered data is
  inherently time-sensitive).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@Jayanth-reflex
Copy link
Copy Markdown
Author

@koala73 Thanks for the thorough review. All four issues addressed in 0d24bd6:

🔴 P1 — Date.now() frozen in CDN cache

Went with the minimal fix: the generated route handler now sets X-No-Cache: 1 on the Response when time_range is present. The gateway converts this to Cache-Control: no-store, bypassing the static tier's s-maxage=600. Unfiltered requests (no time_range) continue to benefit from full CDN caching.

if (body.timeRange) responseHeaders["X-No-Cache"] = "1";

🔴 P1 — timeRange required → optional

Changed timeRange: string to timeRange?: string in both generated client and server interfaces. Also simplified the dead-code branch:

// Before (else branch never executes — effectiveAt is always string from proto):
const ts = typeof e.effectiveAt === 'string' ? Number(e.effectiveAt) : (e.effectiveAt as unknown as number);
// After:
const ts = Number(e.effectiveAt);

🟡 P2 — onTimeRangeChanged fires unconditionally

Now guarded so it only fires when the sanctions panel is actually mounted:

onTimeRangeChanged: () => {
  if (this.state.panels['sanctions-pressure']) {
    void this.dataLoader.loadSanctionsPressure();
  }
},

🟡 P2 — Shared circuit breaker failure state

Introduced a dedicated filteredBreaker for time-range requests with independent failure counters. The canonical breaker (for 'all') and filteredBreaker (for '1h', '7d', etc.) now have completely separate CircuitState, so failures on one path cannot put the other into cooldown.

const breaker = createCircuitBreaker<SanctionsPressureResult>({
  name: 'Sanctions Pressure',
  cacheTtlMs: 30 * 60 * 1000,
  persistCache: true,
});
const filteredBreaker = createCircuitBreaker<SanctionsPressureResult>({
  name: 'Sanctions Pressure (filtered)',
  cacheTtlMs: 10 * 60 * 1000,
  persistCache: false, // filtered data is time-sensitive, no point persisting
});

Both typecheck and typecheck:api pass cleanly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

trust:caution Brin: contributor trust score caution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: sanctions panel ignores timeRange filter — newEntryCount always 0

3 participants